Add job categories - #3239

@timcassell
One thing worth knowing since it isn't obvious from the diff: the old attribute's "adds to categories already defined in code" behavior is gone along with it. Categories now come from exactly one place per job whichever attribute or fluent call defined that job.

Hi @timcassell, please any updates on this PR?

Thanks for the PR! I went through the diff (merge-base b6d2177 .. 1fc3f7f), built the branch, and ran the unit suite — 1082 passed / 0 failed, including all 25 new JobCategoryTests. The findings below were reproduced against that build.

Categories affect job identity but not the job name

src/BenchmarkDotNet/Jobs/MetaMode.cs:20

CategoriesCharacteristic is created with CreateHiddenCharacteristic, which only hides it from the presenters (IsPresentableCharacteristic). JobComparer.Compare walks GetAllCharacteristics(), so it does compare categories.

The result is that two jobs differing only by category survive Distinct(JobComparer.Default) but produce byte-identical benchmark names:

ManualConfig.CreateEmpty()
    .AddJob(Job.Dry.WithCategory("a"))
    .AddJob(Job.Dry.WithCategory("b"))

// two cases, both named:
// B.M: Dry(IterationCount=1, LaunchCount=1, RunStrategy=ColdStart, UnrollFactor=1)

Without a JobCategoryFilter in play, the user silently runs the same job twice, gets two indistinguishable summary rows, and any artifact keyed on the benchmark name (EventPipe traces, per-benchmark exports) collides.

JobsWhichDifferOnlyByCategoriesAreNotConsideredDuplicates currently locks this behavior in. Two ways out: exclude the characteristic from JobComparer, or merge categories when deduplicating.

WithCategories() with an empty array silently doubles the run

src/BenchmarkDotNet/Jobs/JobExtensions.cs:343

The assignment is unconditional, so an empty array still sets the characteristic:

Job.Default.WithCategories().Categories.Count           // 0
Job.Default.WithCategories().HasValue(CategoriesCharacteristic)  // true

So AddJob(Job.Default).AddJob(Job.Default.WithCategories()) yields two DefaultJobs and runs the benchmark twice. That makes job.WithCategories(userSelection.ToArray()) a hazard whenever the selection happens to be empty. Suggest skipping the assignment (or clearing the characteristic) when the deduplicated list is empty.

Same line: WithCategories(null) is legal via params, and currently surfaces as ArgumentNullException: ... (Parameter 'source') thrown from Distinct inside MetaMode.Unique. A guard naming categories would be friendlier.

NullReferenceException on Categories = null

src/BenchmarkDotNet/Attributes/Jobs/JobConfigbaseAttribute.cs:32

Categories.Length is dereferenced without a null check. [DryJob(Categories = null)] is valid C# for a named attribute argument of array type, and throws a bare NullReferenceException out of BenchmarkConverter.TypeToBenchmarks during discovery. Categories is not { Length: > 0 } handles both cases.

Memoized Config silently drops later Categories assignments

src/BenchmarkDotNet/Attributes/Jobs/JobConfigbaseAttribute.cs:30

Config memoizes into config while Categories remains publicly settable, so any assignment after the first Config read is dropped (read Config, set Categories = ["late"], re-read → 0 categories on the same instance). Relatedly, job.WithCategories(Categories) replaces, so a user-derived attribute whose base job already carries categories loses them when a consumer sets Categories. Making Categories init-only, or using append (WithCategory) semantics, would avoid both.

Unrelated whitespace churn

src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs:242

After 92f6955, the only change left in this file is two deleted blank lines around copy.Apply(mutatorJob). Worth reverting so the PR touches nothing it doesn't need.

(The underlying behavior change here is fine — Apply skips characteristics the mutator has no value for, and [DryJob(Categories = ["a"])] + [GcServer(true)] correctly yields gcServer=True cats=[a], so removing the explicit preservation code was safe.)

Checked and correct

Categories are properly excluded from JobIdGenerator, FolderPresenter, SourceCodePresenter (nothing leaks to the child process) and from JobCharacteristicColumn.AllColumns, so the "no effect on id/folder/summary" intent holds. WithCore copies rather than mutating the shared static jobs; AddCategories/Unique dedupe case-insensitively keeping the first spelling; JobCategoryFilter's empty-array behavior matches the existing AnyCategoriesFilter; and collection expressions in attribute arguments (Categories = ["a", "b"]) do compile.

This review was generated by Claude (Opus 5) via Claude Code and posted by @timcassell. The findings were reproduced against a local build, but please treat the analysis as a starting point rather than a verdict.

Hi @timcassell the finds you outlined have now been fixed.

Addressed the findings around job categories, deduplication, null handling, and configuration mutability.

• Job identity & category merging: MetaMode.Categories was incorrectly affecting job identity because hidden characteristics are still considered by JobComparer. The comparer now excludes the categories characteristic, while GetRunnableJobs groups equivalent jobs and merges their categories. For example, jobs with categories "a" and "b" now collapse into a single job containing both categories instead of silently losing one.
• Empty categories: WithCategories() no longer creates a separate job. Empty categories are stored as null, allowing deduplication to work correctly while preserving the documented override behaviour.
• Null handling: Categories now safely handles null. The nullable-array definition also prevents the previous NullReferenceException scenario. The test uses null! to reproduce the same IL where nullable reference types are disabled.
• Configuration mutability: Categories is now init-only, preventing late assignments from being silently ignored by the memoized configuration.
• Argument validation: WithCategories(null) now throws ArgumentNullException with categories as the parameter name.
• Cleanup: Reverted unrelated whitespace changes. The only remaining ImmutableConfigBuilder change is the intentional grouping and category-merging logic.

Re-reviewed at c5478c88. All five points from the earlier comment are addressed — four verified fixed by execution, one partly. Build is clean across all TFMs (netstandard2.0 picks up the repo's ArgumentNullException.ThrowIfNull polyfill), unit suite 1087 passed / 4 skipped / 0 failed, JobCategoryTests 30 passed. The explanatory comments you added at each fix are genuinely good.

Taking both routes for the identity problem — skipping the characteristic in JobComparer and merging in ImmutableConfigBuilder — was the right call; either alone would have left a hole. Worth noting why the pairing is safe: JobComparer.GetHashCode is obj.Id.GetHashCode(), and categories are excluded from JobIdGenerator, so collapsed jobs share an Id and land in the same GroupBy bucket. Had categories leaked into the Id, the merge would have silently never fired. Whitespace churn is gone (git diff -w is byte-identical to the plain diff). MergeCategories's short-circuit can't be wrong, only conservatively false — a needless copy when categories overlap.

Four things left, none blocking:

1. A mutator carrying categories wipes the target's categories. AddJob(Dry.WithCategory("keep")) + AddJob(Default.WithGcServer(true).WithCategory("mut").AsMutator()) → Dry[mut]; "keep" is gone, because copy.Apply(mutatorJob) treats the characteristic as an ordinary overwriting one. That's the same "selecting by a category matches nothing" failure MergeCategories was written to prevent, reached via the mutator path. Pre-existing to this round rather than introduced by it, and it only bites users who put categories on a mutator. Marking the characteristic IgnoreOnApply (as IsMutatorCharacteristic is) and merging explicitly would close it.

2. Meta.Categories = null still throws ArgumentNullException("source") — the unhelpful message the new guard was added to remove. The guard is only in the WithCategories extension; the property it delegates to is public. AddCategories(null) has the same shape.

3. WithCategory(null) silently stores a null category — items=[<null>], and it survives merging. The array is guarded, the elements are not; a null in the list is a latent NRE for anything that formats or compares categories.

4. JobCategoryFilter has no attribute and no CLI option. The predicate itself is consistent with the rest of the codebase — every filter is inclusion-only, and this one is a near-verbatim AnyCategoriesFilter with Descriptor swapped for Job.Meta. What is inconsistent is the surface: AnyCategoriesFilter has both AnyCategoriesFilterAttribute and --anyCategories, AllCategoriesFilter likewise, but JobCategoryFilter has neither, so the only way to use it is constructing it by hand in a config. Given the headline use case is "run only the net8 job", the missing CLI option is the notable one, and it is much cheaper to add before the API ships.

One related design question worth settling explicitly, since the docs are the place users will look: applying JobCategoryFilter drops every case whose job has no categories. That is fine in the documented example where all three jobs are categorized, but in a config mixing categorized and uncategorized jobs, selecting one category silently removes the uncategorized baseline. Either document that, or adopt the convention that a job with no categories always passes.

Reviewed by Claude (Opus 5), posted by @timcassell.